Exposed spatial predicates over Geometry
#366
Conversation
| /// If OGR is built without the GEOS library, this function will always return `false`. | ||
| /// | ||
| /// See: [`OGR_G_IsValid`](https://gdal.org/api/vector_c_api.html#_CPPv413OGR_G_IsValid12OGRGeometryH) | ||
| pub fn is_valid(&self) -> bool { |
There was a problem hiding this comment.
is there a way to determine if OGR was build with GEOS?
There was a problem hiding this comment.
@jdroenner I'll look into it. It might require build.rs calling into gdal_sys, which might be weird...
There was a problem hiding this comment.
If build.rs can find gdalinfo in the path:
There was a problem hiding this comment.
@jdroenner I assume the implication of your question is that we'd want to optionally enable these GOES-dependent features? How about the case when someone compiles with GEOS support, but at runtime a different GDAL build is dynamically linked?
There was a problem hiding this comment.
Yeah, I don't think we should gate these methods on GEOS. GDAL always has them, we should probably match that.
There was a problem hiding this comment.
we should probably match that
WDYM? Assume GEOS is there? (Pretty sure you can build without it, although really rare).
If so, should I remove the caveat in these doc comments?
There was a problem hiding this comment.
I meant letting them compile and returning what GDAL returns.
There was a problem hiding this comment.
I don't want a feature gate. However there should be a method you can call (at runtime) to get the information if GEOS was build in.
There was a problem hiding this comment.
ah i guess its GDALVersionInfo and then looking if GEOS is in there. Maybe we can add a convenience method for that to our VersionInfo struct.
There was a problem hiding this comment.
Great idea. On it! 🤠
| /// Determine if GDAL is compiled with [GEOS](https://libgeos.org/) support. | ||
| pub fn has_geos() -> bool { | ||
| version_info("BUILD_INFO").contains("GEOS_ENABLED=YES") | ||
| } |
There was a problem hiding this comment.
@jdroenner @lnicola Should this be memoized somehow? I could imagine someone incorporating this into their logic (branching on capabilities) and this killing performance.
There was a problem hiding this comment.
I don't think it's necessary (I mean, what are you gonna do if these don't work?), but you can use https://docs.rs/once_cell/latest/once_cell/ if you want to do it.
There was a problem hiding this comment.
Good point. Should probably keep it simple for now. I'll update the docs to reference it and re-request a review for final approval.
There was a problem hiding this comment.
PS: Plan on continuing the work of fleshing out the vector API in subsequent PRs.
|
bors d+ |
|
✌️ metasim can now approve this pull request. To approve and merge a pull request, simply reply with |
Yes, I will do that. I wanted to replicate this pattern of files with additional but related inherent methods to make it easier to maintain, document and test. e.g. set-theoretic operations. |
Ooops, didn't realize this was out there! |
|
bors r+ |
366: Exposed spatial predicates over `Geometry` r=metasim a=metasim - [x] I agree to follow the project's [code of conduct](https://github.com/georust/gdal/blob/master/CODE_OF_CONDUCT.md). - [X] I added an entry to `CHANGES.md` if knowledge of this change could be valuable to users. --- Partially addresses #306 cc: `@thomas001` Co-authored-by: Simeon H.K. Fitch <[email protected]>
|
Build failed: |
|
bors retry |
|
Build succeeded:
|
CHANGES.mdif knowledge of this change could be valuable to users.Partially addresses #306
cc: @thomas001